Mobile UX consolidated pass#4503
Open
iscekic wants to merge 164 commits into
Open
Conversation
Even a disabled organizations.list observer surfaces the shared cache entry's error state, so an org-list failure rendered the personal scope entry as a full-screen error although every personal securityAgent call succeeds. The role query now masks query state entirely for the personal scope, where the org list is irrelevant.
…ped to the POST
- SheetHeader was invisible on formSheet pickers: react-native-screens lays
out a formSheet's scroll view natively and only honors a header when
[header, scroll view] are the screen content's direct children — the
wrapping View made it pin the list over the header. PickerSheet drops the
wrapper (fragment) and marks the header collapsable={false} so view
flattening can't recreate the bug; mode-picker migrates to PickerSheet.
Verified on-device: mode + model pickers now show title + Done.
- device-auth: the 15s start timeout now clears as soon as the POST
resolves. start() stays suspended on the auth browser await for as long
as the sheet is open, so the timer previously fired mid-sign-in, stomped
the live pending/idle state with a bogus error, and left a zombie poll
running behind it.
The bots.length !== 1 guard already narrows bots[0] to ActiveMemberRow; the root oxlint config forbids the non-null assertion and flags it as unnecessary.
…event-driven clear - session mutations: dedup only an ADJACENT identical op (repeat of the chain tail) instead of any op in a global set, so rename A -> B -> A runs the final A and the user's latest intent wins; double-taps still coalesce - kilo-chat redeliver (DO): reserve the webhook-chain slot synchronously with no external await first, so a concurrent request can't slot a later message ahead of the redelivery or run delivery out of order; the redelivered event is pushed inside the serialized task, strictly before deliverToBot - kilo-chat redeliver (client): drop the optimistic clear + guarded restore + invalidate; the server's message.redelivered event is the single source of truth for the flag, which removes both the rollback ambiguity on an unclear HTTP error and the refetch race that could overwrite a newer delivery failure with a stale pre-failure snapshot
…via event pushEventToHumanMembers is best-effort, so a requesting client whose socket dropped would miss message.redelivered and keep showing Retry indefinitely even though the server cleared the flag and retried. Apply the clear on HTTP success as a targeted single-message cache write (not optimistic, not a refetch); a strictly-later message.delivery_failed event still restores failure if the retry ultimately fails.
- ScreenHeader: drop unused backLabel/fallbackHref props (zero call sites) - QueryError: unify message/description into one prop (message), delegate its shell to EmptyState with a bubble-chrome override instead of duplicating layout; keep permission/not-found variants (still used by review-detail-screen) - PickerSheet: replace the generic `fallback` prop with a single `expired` boolean, folding the repeated "Options expired" EmptyState into the component itself - StatusDot: drop the always-true `glow` prop - route-params: collapse parseParam's two overloads into one generic signature with a default type param
Pre-existing EmptyState call sites keep their previous screen-reader behavior; only QueryError (which had accessibilityRole='header' before delegating to EmptyState) opts in via titleAccessibilityRole.
- use-session-mutations: replace hand-rolled per-key promise chain with the shared chainSave primitive (already used by use-model-preferences and use-code-reviewer); adjacent-op dedupe is dropped since rename/delete both go through a confirm step that already prevents double-fire - shrink onMutate handlers to single-expression returns - delete hasFailedAttachments (one-line wrapper) and inline its .some() at the only call site - remove five dead toast guards in agent-chat/new.tsx's handleCreate; they're already covered by the Start button's canCreate disabled state - drop the toast on pull-to-refresh failure for the session list; the inline "Could not refresh" header line already owns that feedback, which lets refetch revert to Promise<void> across the three call sites
…, dedupe refresh/text helpers - Replace the bespoke rename-conversation formSheet route + sheet component with RenameModal (already used for kiloclaw instance and org renames); delete the route, sheet, layout registration, and path helper. RenameModal gains a maxLength prop (default 50) so conversation rename can pass 200. - Delete a use-messages test asserting the abandoned resend-as-new-message retry design; production handleRetrySend only redelivers in place. - Inline the conversation-route "Failed to load conversation" message and delete the now-unreachable not-found branch of getConversationRouteErrorMessage. - Drop the unused memberContext field from RedeliverMessageResult's ok variant. - Extract a shared contentBlocksToText(content, separator) helper for the three near-identical reply/auto-title text extractions in conversation-do.ts and services/messages.ts. - Extract a useManualRefresh hook for the three duplicated pull-to-refresh blocks (conversation list, kiloclaw instance list, instance dashboard).
Review follow-up: ConversationRow hand-rolled the renaming state + open/close/save pattern that useConversationRename already encapsulates — wire it onto the hook. Also update the stale useRenameConversation comment (the rename form sheet is gone; callers now rename via RenameModal) and narrow useConversationOptionsSheet's return to what its caller uses.
…ut, dedupe org-id/pending helpers - InstanceContextBoundary now owns its own header+background shell (title prop) instead of relying on dead children passthrough; all 11 callers collapse to a one-line early return. - Move the provisioning wall-clock timeout out of the onboarding machine into local ProvisioningStep state — it had no other producer/consumer and was threaded through three components just to flip one flag. - Restore single-shell + body-switch structure in changelog/channels/secrets screens instead of duplicating the header per branch. - Replace hand-rolled pending-id state + onSettled clearing with React Query's mutation.variables (model-list, device-pairing), matching the pattern already used by exec-policy. - Extract instanceOrgId() helper for the organizationId-from-context dance repeated across ~10 screens. - Collapse REDEPLOY_BLOCKING_STATUSES into a canStart-derived check. - Invalidate billing status via queryClient in the mutation's onSuccess instead of threading an onContinued callback through three components. - Collapse useKiloClawBillingStatus's number|function refetchInterval union to a single function-typed param. - Replace model-list's remount-key search-clear hack with TextInput.clear().
Adds [platform]/_layout.tsx to validate the scope+platform route params once for all six code-reviewer platform routes, instead of each route calling useValidatedReviewerRouteParams() and branching on InvalidRouteState itself. The five config-editing routes (style, gate, focus-areas, repos, instructions) move into an (edit) route group whose own layout runs useReviewerEditGuard once; the group adds no URL segment so existing links are unaffected. Drops the forced Route/RouteContent split in every route now that there's no null check to gate on.
Content half of the previous commit's route restructure: the layout now does the validation/guard work, so each of the six route files just reads the already-validated params via useLocalSearchParams instead of re-running useValidatedReviewerRouteParams()/useReviewerEditGuard and branching into a Route/RouteContent split.
…Rows pick() and capitalize() each existed verbatim in 2-4 places; both now live once in @/lib/utils and every call site imports the shared version (finding-row's severityLabel was exactly capitalize). bitbucket-overview.tsx's hand-rolled overview rows are now built with buildOverviewRows(PLATFORM_CAPABILITIES.bitbucket, ...), matching what it already produced for bitbucket's capability flags; resolveRowOnPress moves alongside buildOverviewRows in platform-overview-rows.ts so both overview screens share one copy instead of two identical locals. Also deletes platform-overview-screen.tsx's bitbucket+personal "Not available" EmptyState: the sole caller gates on parseReviewerPlatform, which already returns null for that combination, so the branch was unreachable re-validation of an already-trusted route param.
dismiss-finding-screen.tsx hand-rolled the exact bg-secondary/border-b radio list that PillGroup already renders for the automation and notification severity pickers. Widens PillGroup's value to T | null (the dismiss form starts with no reason selected) and its options to a readonly array so an `as const` literal list can be passed directly.
…spread
advanceSettingsBaseline(baseline, patch) was { ...baseline, ...patch }
with a JSDoc comment and its own test block. Inlines the spread at all
five settings-screen call sites and drops the function and its tests.
…y-agent Moves platform-error-screen.tsx out of components/code-reviewer/ (a neutral location, since security-agent now imports it too) and adds optional eyebrow/errorTitle/message/variant/isRetrying so it covers both features' full-screen load-failure shape instead of each screen hand-rolling its own ScreenHeader+QueryError wrapper. Replaces the duplicated wrapper at 7 security-agent sites (analysis, automation, notification, repository, sla, settings-overview, scope-entry) with PlatformErrorScreen, preserving each site's existing icon/title/message via explicit variant/errorTitle props. scope-entry-screen.tsx's local ScopeEntryError component is deleted as it's now just this shared one.
Rest of the previous commit's change: generalizes PlatformErrorScreen with optional eyebrow/errorTitle/message/variant/isRetrying and points the 7 security-agent full-screen error states (plus code-reviewer's existing 3 call sites) at it instead of each hand-rolling its own ScreenHeader+QueryError wrapper.
…lled rows RepoToggleRow (new, components/repo-toggle-row.tsx) replaces the copy-pasted repo checkbox row (Lock + name + Check, identical classes and toggle logic) in code-reviewer's repos.tsx and security-agent's repository-settings-screen.tsx; it's built on ChoiceRow via a new optional `children` override (for the icon-prefixed label ChoiceRow's plain label/description text can't express). Also adopts ChoiceRow at the remaining hand-rolled Pressable+Check rows this PR left duplicating its boilerplate: the repository-mode radio picker in both repos.tsx and repository-settings-screen.tsx, and finding-filter-modal's FilterOptionRow (kept as a thin wrapper using ChoiceRow's children override, since its default label rendering's `capitalize` class would mangle arbitrary repo full names).
…t directly dashboard-screen.tsx's "More actions" button opened a one-item action sheet (View audit report / Cancel) for what scope-entry-screen.tsx and settings-overview-screen.tsx already do as a direct MoreHorizontal press. Extracts the shared Pressable into AuditReportButton and points all three at it, dropping dashboard's action sheet plumbing for this action (its repo-filter action sheet is unaffected). Sanctioned behavior change: tapping the button on the dashboard now opens the audit report directly instead of via the action sheet.
sla-settings-screen.tsx had four parallel useState(Number.NaN) hooks plus hand-built setters/values Records used only to route SlaDayRow's per-severity value/onChange through the four fields by key. Collapses to one useState<Record<SlaSeverity, number>>, so the day-row list reads and writes slaDays[row.key] directly instead of indirecting through the Records.
…gentCapability useSecurityAgentEditCapability was just useSecurityAgentCapability(scope) .canManage, and useSecurityAgentOrgRole's only caller (finding-detail- screen.tsx) took the raw role only to re-run canManageSecurityAgent(scope, role) — re-deriving what useSecurityAgentCapability already returns. Deletes both hooks, keeping useSecurityAgentCapability as the one query; updates all 7 callers. Also drops the now-unused OrganizationRole re-export from lib/security-agent.ts (knip-flagged after the callers that used it were removed).
…branch The !data.success branch rendered byte-identically to the transient "could not load" branch above it (same title/variant/onRetry) — folds into one if (!data || !data.success) after the permanent-error-code check, which still returns early with the not-found/permission variant untouched.
…creens The pre-sharing platform-error-screen hardcoded eyebrow="Code Reviewer" on its ScreenHeader; the shared PlatformErrorScreen defaults eyebrow to undefined, so platform-overview-screen's three error states (permission, provider-state, config) silently lost it. Pass it explicitly at all three call sites.
…ntry consent chain OrganizationBoundary now calls useOrgBoundary() itself and takes an optional title, collapsing all 7 call sites to a single line. Purchase/restore error suppression drops the hand-threaded suppressToast option in favor of a mount counter (useInlinePurchaseErrorOwnership) that the subscription screen registers once. FormField owns blur-validation via a validate prop, removing ~4 copy-pasted touched-field state blocks. sentry-consent drops the manual gate/Promise-constructor plumbing for a plain await chain with identical serialization/no-reject/un-mark-on-failure semantics. Also: extracted AddCreditsRow and InlineRetry to de-duplicate zero-balance and retry UI, removed force-update's redundant Copy URL button, folded poll-response's 1xx/3xx fallthrough into the error arm, dropped formatDate's no-op options object, and trimmed validator/utils tests down to the product rules and Hermes-specific behavior they actually cover.
…e-error owner count in tests The low-balance-alert sheet's folded loading branch used isLoading, which is false for an offline paused fetch (no data, no error) — the sheet rendered nothing. Use isPending (no data AND no error) so the paused case shows the skeleton while real errors still reach QueryError; guard with organizationId so a disabled query still falls through to OrganizationBoundary. Also add resetInlinePurchaseErrorOwnership mirroring the toast-dedup reset and wire it into the test beforeEach so a throwing test can't leak ownership into later tests, and drop the pointless call-site alias for the ownership harness (the rules-of-hooks/new-cap dodge now lives inside the helper, matching the existing renderProviderElement pattern).
…led keys The tail promise was registered after an await, so back-to-back saves for the same key could read a stale predecessor and race instead of chaining in order; the in-flight map also never released keys once their chain settled. Register the tail synchronously and delete it once it settles, guarded so a newer tail is never clobbered.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the consolidated mobile UX pass: every finding from the mobile UX audit (dead ends, eternal skeletons, invisible errors, missing CTAs, tab-bar/inset defects, disabled-state lies, silent mutations, a11y gaps), built as shared primitives first, then per-domain migrations, then cross-app sweeps.
Correctness & privacy (Phase 0)
parseTimestampeverywhere (Hermes-safe); code-reviewer domain failures ({success:false}) now route toonErrorShared primitives (Phase 1)
TabScreenScrollView/useTabBarBottomPadding(single source for tab-bar clearance, ~40 screens migrated),useDetailScreenBottomPadding,QueryErrorvariants + placement + retry spinner,EmptyStateplacement,Button44pt targets +loading,ConfigureRowdisabled/chevron truth,SheetHeader/PickerSheet, sharedRenameModal,openExternalUrl(P7),FormField(uncontrolled, a11y error announcement),ChoiceRow(radio/checkbox semantics, single haptic owner),SpinningIcon+ reduced-motion support (incl. skeleton shimmer), toaster ordering + Kilo-token styling, form-sheet detent helpers.Domain passes (Phases 2–6)
canCreategating, session-detail recovery + message-shaped skeletons, picker fallback headers + synchronous bridge init, kilo-chat cached-data-wins routing, token-failure retry, inline rename errors, bot-offline CTA, message retry-send (resolves the failed original), favorites with latest-write-wins optimistic updates, behaviorally-modal reaction picker, single creation affordance on empty surfacesisLoading || !datatraps), org boundary discriminates transient failure from stale org (retry vs re-select), sheets own their errors inline (toasts render behind iOS form sheets), field validation viaFormField, role guards fail closed, role-aware empty-state CTAs, notifications-card failure feedback, forced-update store fallbacks, Kilo Pass inline feedback + StoreKit timeout, consent card caught mutations + pending states, login field labelingOptionListsaves await outcomes with serialized config writes, review list/detail states (UNAUTHORIZED/FORBIDDEN/NOT_FOUND classified as permanent, no un-retriable retry)Cross-app sweeps (Phase 7)
Empty/error placement per convention (centered full-screen with tab clearance; top-aligned inline), disabled-vs-loading visual contract, skeleton geometry matched to loaded content, press feedback on inert-feeling controls, KVRow overflow, status colors migrated to Focus tokens (new
infofamily added in the same style), sentence-case copy pass,formatDateconsolidation.Product notes
@kilocode/app-sharedhelpers with Intl grouping:$1,234.50(was$1234.50at a few residual sites). Signed off.toLocaleDateStringsites consolidated onto sharedformatDate, which follows the device locale (numeric date style). Four sites moved from long month names to the numeric form.--muted-foreground#7a756b#6f6a61--destructive#c25647#be4e3f--good#2f9a5f#278150--warn#b27214#9f6612--inputborder (light)--inputborder (dark)Foreground-on-fill pairs (
--good/--warn/--destructivebadges) all clear 4.5:1 as well; dark text tokens already passed and were left unchanged. Tinted tile tokens updated to match. RootDESIGN.mdnow documents that mobile intentionally uses the Focus palette.Verification
pnpm format && pnpm typecheck && pnpm lint && pnpm check:unusedclean; 530 mobile + 192 app-shared + 91 kilo-chat-hooks tests green||→??autofix regressions, no backend-stringnew Date(), error-before-skeleton branch ordering everywhere, single feedback owner per mutation (no double toast+inline, no silent mutations)kilo-chat-hookschanged (injected package) — re-inject and clear the Metro cache before device testing.Known follow-ups (deliberately left)
flyRegioninstances (crossfaded, not popped)Linking.openURL(system handoff arguably better UX than the in-app browser helper)useConnectFlowdedupe remain deferred per the planUpdate — review round + second e2e pass (2026-07-12)
Review comments (all 18 kilobot findings valid, fixed, threads resolved)
redeliverMessageop on the conversation DO — re-enqueues delivery of the existing row (no new message, no attachment re-linking), clearsdelivery_failed, publishesmessage.redeliveredbefore enqueueing the attempt (so a fresh failure can't be clobbered by a delayed clear), requires exactly one eligible bot (no fake success with zero bots, no double-delivery with many). Fixes the "attachment already linked" and duplicate-row findings.Sentry.close()before re-init, serialized through a chain promise that never rejects (a failed transition no longer poisons later ones); the applied-consent ref is un-marked on failure so the next change retries teardown.null(unpinned) as valid; instance controls block Start/Redeploy duringrecovering/restoring; security-agent scope list keeps Personal reachable on an org-list outage (and the personal scope entry no longer inherits the shared org-list error); remediation cancel distinguishes "Cancellation requested" from "Remediation cancelled"; onboardinginstance_stoppedretry actually starts the instance; device-auth cancel aborts the in-flight POST.Fixes from the second on-device pass (iOS)
react-native-screensonly honors a formSheet header when[header, scrollview]are the screen content's direct children; removed the wrapper View and marked the headercollapsable={false}. Verified on-device.start()including the auth-browser await, firing mid-sign-in and stomping state with a bogus error + zombie poll; now cleared as soon as the POST resolves. Verified on-device.accessible={false}fix as the filter sheet); session search retry; provisioning pulse gated on Reduce Motion; header touch targets to 44pt via hitSlop; kiloclaw copy sentence-case + pairing-refresh a11y label.Verification
format/typecheck/lint/check:unused) across apps/mobile; 530 mobile + 91 kilo-chat-hooks + 107 kilo-chat pkg + 415 kilo-chat service tests pass.